Moving tests from test/ to python/ray/tests/ by williamma12 · Pull Request #3950 · ray-project/ray

williamma12 · 2019-02-05T07:52:51Z

What do these changes do?

Moves relevant python tests to python/ray/test/ so that it will ship with ray when installed from Pypi.

Related issue number

williamma12 · 2019-02-05T07:54:24Z

Using this comment to start a discussion on the motivations behind why certain tests were moved and others were not

Moved:

actor_test.py: mainly just tests that the python api passes the arguments and other common class functionality to ray properly
array_test.py: Tests ray.experimental.array functions work as expected
cython_test.py: Tests that cython works as expected with ray
failure_test.py: Tests that ray propagates errors correctly
mini_test.py: Tests basic ray python api calls
recursion_test.py: Tests recursive python functions work as expected
runtest.py: Tests various ray’s python api functionality
tensorflow_test.py Tests that tensorflow works as expected

Not Moved:

autoscaler.py: Small stress test
component_failures_test.py:Fault tolerance testing for each of the different parts of ray
credits_test.py: Tests redis
debug_tools_test.py: Tests that the debug tools still work
microbenchmarks.py: Benchmarking
jenkins_tests/: Stress tests
monitor_test.py: Tests that the monitor returns the expected values
multi_node_test.py: Tests that the nodes behave as expected
multi_node_test_2.py: More tests that tests that nodes behave as expected
node_manager_test.py: Tests the node manager works for edge cases
object_manager_test.py: Tests that the object manager works as expected
stress_tests/ and stress_tests.py: Stress tests
tempfile_test.py: Tests that the temporary files are created properly

robertnishihara · 2019-02-05T07:55:41Z

Looks like it should be tests not test. See https://python-packaging.readthedocs.io/en/latest/testing.html.

robertnishihara · 2019-02-05T08:04:13Z

Thanks @williamma12! I think we should actually move all of the tests except stress_tests/* and jenkins_tests/.

robertnishihara · 2019-02-05T08:04:40Z

@pcmoritz what do you think?

pcmoritz · 2019-02-05T08:15:24Z

I agree and we can also rename the "test" directory in the root to "ci" or "integration" something else so it is clear that tests should not go there, but into the new folder instead.

AmplabJenkins · 2019-02-05T08:20:52Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11568/
Test FAILed.

AmplabJenkins · 2019-02-06T07:46:15Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11607/
Test FAILed.

williamma12 · 2019-02-06T07:46:55Z

Sounds good! I've moved all the files from the old test/ to python/ray/test except for stress_tests/, jenkins_tests/ ~~, and microbenchmarking.py. microbenchmarking.py was not moved because it does not actually contain any pytest tests but just benchmarks that prints the benchmarks.~~ I was mistaken, microbenchmarking.py does have pytest tests

I have also updated the folder names from test/ and python/ray/test/ to integration/ and python/ray/tests, respectively.

Also, I am open to suggestions for better names for the new integration/ folder!

AmplabJenkins · 2019-02-06T08:33:34Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11609/
Test FAILed.

AmplabJenkins · 2019-02-06T18:32:39Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11618/
Test FAILed.

AmplabJenkins · 2019-02-06T20:23:39Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11622/
Test FAILed.

AmplabJenkins · 2019-02-07T05:50:28Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11633/
Test FAILed.

AmplabJenkins · 2019-02-07T06:13:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11634/
Test FAILed.

AmplabJenkins · 2019-02-07T21:25:51Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11658/
Test FAILed.

AmplabJenkins · 2019-02-07T21:37:33Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11661/
Test FAILed.

AmplabJenkins · 2019-02-08T01:16:03Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11671/
Test FAILed.

robertnishihara · 2019-02-08T22:22:52Z

ci/jenkins_tests/multi_node_tests/many_drivers_test.py

rename to utils

AmplabJenkins · 2019-02-08T23:56:06Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11708/
Test FAILed.

AmplabJenkins · 2019-02-09T00:18:59Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11711/
Test FAILed.

AmplabJenkins · 2019-02-10T22:17:16Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11744/
Test FAILed.

raulchen · 2019-02-11T03:18:16Z

.travis.yml

I think now it's a good chance to follow the test discovery convention (see https://docs.pytest.org/en/latest/goodpractices.html#conventions-for-python-test-discovery) and change these file-based commands to to one single directory-based command (or a few sub-directories if needed).
The benefit is that we can only have one statement in this travis ci file, e.g., pytest -v --duration=10 python/ray/tests. And when someone adds a new test file, they don't need to modify here.

Thanks! Doing so also revealed that the test_queue.py lacked a teardown_module, which I added in #4012.

I notice that there was also a comment about object_manager_test.py and I left it as a separate command but I am open to suggestions

AmplabJenkins · 2019-02-11T03:54:26Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11762/
Test FAILed.

raulchen · 2019-02-12T06:09:15Z

.travis.yml

I think this will run test_object_manager.py twice, right? Maybe we can move the above comment to the test_object_manager.py file, and don't need to make this a separate command. cc @guoyuhong

I reworded the comment so that I could keep the comment in .travis.yml and delete the line so test_object_manager.py won't be ran twice

AmplabJenkins · 2019-02-14T03:13:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11917/
Test FAILed.

ci/travis/install-dependencies.sh

AmplabJenkins · 2019-02-14T06:00:42Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/11927/
Test FAILed.

ci/travis/install-dependencies.sh

AmplabJenkins · 2019-02-17T22:22:15Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12055/
Test FAILed.

robertnishihara · 2019-02-17T23:35:45Z

python/ray/tests/test_recursion.py

@williamma12 does this test actually work in pytest?

oops good catch. Did not intend to move the remote function inside the test and undoing it made the test pass.

For consistency, can you rename init to ray_start?

AmplabJenkins · 2019-02-18T00:21:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12057/
Test FAILed.

AmplabJenkins · 2019-02-18T00:27:29Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12058/
Test FAILed.

AmplabJenkins · 2019-02-18T00:46:16Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12060/
Test FAILed.

AmplabJenkins · 2019-02-18T05:45:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12069/
Test FAILed.

AmplabJenkins · 2019-02-18T06:23:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12070/
Test FAILed.

AmplabJenkins · 2019-02-18T18:28:50Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12098/
Test FAILed.

AmplabJenkins · 2019-02-19T17:42:58Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12116/
Test FAILed.

AmplabJenkins · 2019-02-20T02:07:37Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12138/
Test FAILed.

robertnishihara · 2019-02-20T19:03:24Z

There seems to be an issue with test_monitor.py.

AmplabJenkins · 2019-02-20T21:07:48Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12164/
Test FAILed.

AmplabJenkins · 2019-02-20T22:16:34Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12170/
Test FAILed.

AmplabJenkins · 2019-02-20T22:54:14Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12174/
Test FAILed.

AmplabJenkins · 2019-02-20T23:18:56Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12177/
Test FAILed.

AmplabJenkins · 2019-02-21T07:58:16Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12194/
Test FAILed.

AmplabJenkins · 2019-02-21T08:04:47Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12195/
Test FAILed.

robertnishihara

LGTM once tests pass.

AmplabJenkins · 2019-02-21T08:18:00Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12196/
Test FAILed.

robertnishihara · 2019-02-21T19:11:10Z

Jenkins, retest this please.

robertnishihara reviewed Feb 8, 2019

View reviewed changes

ci/jenkins_tests/multi_node_tests/many_drivers_test.py Outdated

Copy link

Collaborator

robertnishihara Feb 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to utils

williamma12 force-pushed the refactor/move_python_tests branch from 7511514 to 870d4e1 Compare February 10, 2019 22:11

raulchen reviewed Feb 11, 2019

View reviewed changes

raulchen reviewed Feb 12, 2019

View reviewed changes

pcmoritz mentioned this pull request Feb 13, 2019

Unify ray discovery during tests #4031

Closed

williamma12 force-pushed the refactor/move_python_tests branch from 22cbc75 to cc7de53 Compare February 14, 2019 03:02

robertnishihara reviewed Feb 14, 2019

View reviewed changes

ci/travis/install-dependencies.sh Outdated Show resolved Hide resolved

robertnishihara reviewed Feb 14, 2019

View reviewed changes

ci/travis/install-dependencies.sh Outdated Show resolved Hide resolved

robertnishihara reviewed Feb 17, 2019

View reviewed changes

williamma12 force-pushed the refactor/move_python_tests branch from 8317566 to 9da8220 Compare February 18, 2019 00:22

williamma12 force-pushed the refactor/move_python_tests branch from 714c73c to 60b8b87 Compare February 19, 2019 03:58

williamma12 force-pushed the refactor/move_python_tests branch from 60b8b87 to df6e0f2 Compare February 20, 2019 02:00

williamma12 force-pushed the refactor/move_python_tests branch from df6e0f2 to 4f0c5d6 Compare February 20, 2019 21:05

robertnishihara force-pushed the refactor/move_python_tests branch from 3a4483d to 4581216 Compare February 21, 2019 07:57

robertnishihara approved these changes Feb 21, 2019

View reviewed changes

williamma12 and others added 2 commits February 21, 2019 00:13

Move python tests to python/ray/tests/.

2c3817e

Add missing test.

1127d6b

robertnishihara force-pushed the refactor/move_python_tests branch from 81f759a to 1127d6b Compare February 21, 2019 08:14

robertnishihara changed the title ~~Moving tests from test/ to python/ray/test/~~ Moving tests from test/ to python/ray/tests/ Feb 21, 2019

robertnishihara merged commit c7a4c74 into ray-project:master Feb 21, 2019

Conversation

williamma12 commented Feb 5, 2019

What do these changes do?

Related issue number

Uh oh!

williamma12 commented Feb 5, 2019

Uh oh!

robertnishihara commented Feb 5, 2019

Uh oh!

robertnishihara commented Feb 5, 2019

Uh oh!

robertnishihara commented Feb 5, 2019

Uh oh!

pcmoritz commented Feb 5, 2019

Uh oh!

AmplabJenkins commented Feb 5, 2019

Uh oh!

AmplabJenkins commented Feb 6, 2019

Uh oh!

williamma12 commented Feb 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented Feb 6, 2019

Uh oh!

AmplabJenkins commented Feb 6, 2019

Uh oh!

AmplabJenkins commented Feb 6, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 7, 2019

Uh oh!

AmplabJenkins commented Feb 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 8, 2019

Uh oh!

AmplabJenkins commented Feb 9, 2019

Uh oh!

AmplabJenkins commented Feb 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 14, 2019

Uh oh!

Uh oh!

AmplabJenkins commented Feb 14, 2019

Uh oh!

Uh oh!

AmplabJenkins commented Feb 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 18, 2019

Uh oh!

AmplabJenkins commented Feb 18, 2019

Uh oh!

AmplabJenkins commented Feb 18, 2019

Uh oh!

AmplabJenkins commented Feb 18, 2019

Uh oh!

AmplabJenkins commented Feb 18, 2019

Uh oh!

williamma12 commented Feb 6, 2019 •

edited

Loading